Repository navigation
FIX: Validate caller-controlled native buffer sizes - #802
Open
gargsaumya wants to merge 52 commits into
Open
gargsaumya wants to merge 52 commits into
gargsaumya wants to merge 52 commits into
Conversation
PR Performance Report
|
| Database task | Unix / SQL Server 2025 |
|---|---|
| Executemany inserts | 23.1% slower |
The largest recorded phase increases for these tasks are shown below. Phase timings are supporting evidence, not root-cause proof.
Coverage: 1 of 2 environments completed. Advisory result; does not block merging.
Unavailable: Unix / SQL Server 2022 (missing).
Measured timings
| Environment | Database task | Before | After | Change |
|---|---|---|---|---|
| Unix / SQL Server 2025 | Executemany inserts | 154.689 ms | 189.666 ms | +23.1% |
Performance diagnostics
Phase times are inclusive diagnostics and must not be added together. They identify where measured time changed, not why it changed.
Unix / SQL Server 2025
Executemany inserts: py::executemany::param_type_detection +29.855 ms; ddbc::BindParameterArray +6.083 ms; py::executemany::cpp_call +4.798 ms.
All database tasks and timings
Unix / SQL Server 2025
| Database task | Before | After | Paired change | Result |
|---|---|---|---|---|
| Connection opening | 98.970 ms | 98.778 ms | -0.3% | no signal |
| SELECT queries | 1.082 ms | 1.077 ms | +1.0% | no signal |
| Row insertion | 33.697 ms | 33.532 ms | +1.7% | no signal |
| Executemany inserts | 154.689 ms | 189.666 ms | +23.1% | consistent slowdown |
| Fetch-all queries | 119.420 ms | 120.165 ms | +0.6% | no signal |
| Row-by-row fetching | 14.015 ms | 14.478 ms | +3.8% | no signal |
| Batched row fetching | 116.380 ms | 117.867 ms | +0.9% | no signal |
| Transaction commit and rollback | 109.860 ms | 109.284 ms | +0.5% | no signal |
| Arrow row fetching | 92.727 ms | 96.893 ms | +4.4% | no signal |
| 100,000-row insertion | 426.477 ms | 421.771 ms | -0.4% | no signal |
| Row fetching in batches of 100 | 120.706 ms | 122.185 ms | +1.6% | no signal |
| Row fetching in batches of 10,000 | 137.597 ms | 127.833 ms | -6.5% | no signal |
| Repeated positional queries | 32.109 ms | 32.258 ms | +0.7% | no signal |
| Repeated named-parameter queries | 34.936 ms | 34.911 ms | +0.0% | no signal |
| Legacy 100,000-row insertion | 347.053 ms | 348.711 ms | +0.8% | no signal |
| Insertion with explicit input sizes | 487.271 ms | 586.595 ms | +19.6% | no signal |
| Joined aggregation queries | 161.771 ms | 161.740 ms | -0.5% | no signal |
| Large joined-result fetching | 192.732 ms | 189.532 ms | -1.3% | no signal |
| 1.2-million-row fetching | 3411.774 ms | 3427.524 ms | +0.3% | no signal |
| Common table expression queries | 5.119 ms | 5.149 ms | +0.9% | no signal |
| 256 KiB VARCHAR(MAX) / fetchall() | 1.493 ms | 1.420 ms | -4.7% | no signal |
| 10,000 scalar values / fetchval() (debug disabled) | 105.390 ms | 108.513 ms | +3.0% | no signal |
Build and measurement details
PR head: 8fa595e760526d063585543b3f129ea0ac216b69
Base: bd0aaf89c724a92c934b68bca41469655bfc733c
Measured merge: ad54d95c74ada8625ca1dedcef3b581fbe86811c
- Unix / SQL Server 2025: Python 3.12.3, x86_64, SQL 17.0.5015.4; 5 paired comparisons and 1 warmup.
A consistent change requires more than 20% median paired movement, at least 1 ms between the median runtimes, and at least 80% of pairs exceeding the relative threshold in the same direction. A slowdown without enough pair agreement is reported as inconsistent.
The displayed change is the median of paired before-and-after ratios. It is not recalculated from the two displayed median runtimes.
Unavailable or rejected data: Linux-SQL2022 (missing)
Both revisions use profiling-enabled builds on the same agent and database, with alternating order and discarded warmups. Results are diagnostic and do not represent production-wheel latency.
Raw samples and logs are attached to the ADO run as profiler-* artifacts.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved row-count and binary validation gaps remain, and an existing Arrow batch-size test requires reconciliation.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (2)
What changed in this PR
Hardens Python-to-native boundary validation for row counts, allocations, metadata, and binary inputs.
Changes:
- Adds row-count, boolean, and input-size validation.
- Adds native allocation overflow and metadata checks.
- Tightens binary and wide-character handling with regression tests.
| File | Description |
|---|---|
tests/test_024_bulkcopy_arrow.py |
Tests boolean batch-size rejection. |
tests/test_004_cursor.py |
Tests cursor size and metadata validation. |
mssql_python/pybind/ddbc_bindings.cpp |
Adds native allocation and parameter validation. |
mssql_python/cursor.py |
Validates row counts and input sizes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Native paths still permit dangerous negative or extremely large allocations, and arrow_reader() does not reject invalid sizes synchronously.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 4
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changes
Summary
mssql_python/cursor.pyLines 58-78 58 MAX_NATIVE_PARAMETER_SIZE: int = 256 * 1024 * 1024
59
60
61 def _encoded_length_exceeds(value: str, encoding: str, limit: int) -> bool:
! 62 encoder = codecs.getincrementalencoder(encoding)(errors="strict")
! 63 total = 0
! 64 chunk_size = 4096
! 65 value_length = str.__len__(value)
! 66 for offset in range(0, value_length, chunk_size):
! 67 end = min(offset + chunk_size, value_length)
! 68 chunk = str.__getitem__(value, slice(offset, end))
! 69 total += len(encoder.encode(chunk, final=end == value_length))
! 70 if total > limit:
! 71 return True
! 72 if value_length == 0:
! 73 total += len(encoder.encode("", final=True))
! 74 return total > limit
75
76
77 def _utf16_length_exceeds(value: str, limit: int) -> bool:
78 bounded_value = str.__getitem__(value, slice(0, limit + 1))Lines 2673-2681 2673 binary_values = [
2674 value for value in column if isinstance(value, (bytes, bytearray))
2675 ]
2676 if c_type == ddbc_sql_const.SQL_CHAR.value:
! 2677 text_is_large = any(
2678 _encoded_length_exceeds(
2679 value,
2680 encoding_settings["encoding"],
2681 MAX_INLINE_CHAR,Lines 2693-2701 2693 requires_row_fallback = bool(
2694 narrow_text_type and text_values and binary_values
2695 )
2696 if narrow_text_type and binary_values and not text_values:
! 2697 c_type = ddbc_sql_const.SQL_CHAR.value
2698 is_dae = text_is_large or binary_is_large
2699
2700 # Sanitize precision/scale for numeric types
2701 if sql_type in (Lines 2721-2729 2721
2722 # For SQL Server VARBINARY(MAX), we need to use large object binding
2723 if max_binary_size > MAX_INLINE_BINARY:
2724 if sql_type != ddbc_sql_const.SQL_SS_UDT.value:
! 2725 sql_type = ddbc_sql_const.SQL_LONGVARBINARY.value
2726 is_dae = True
2727
2728 # Update column_size to actual maximum size if it's larger
2729 # Always ensure at least a minimum size of 1 for empty stringsmssql_python/pybind/ddbc_bindings.cppLines 325-334 325
326 template <typename ParamType>
327 ParamType* AllocateParamBufferArray(std::vector<std::shared_ptr<void>>& paramBuffers,
328 size_t count) {
! 329 if (count > std::numeric_limits<size_t>::max() / sizeof(ParamType)) {
! 330 ThrowStdException("Parameter buffer size is too large");
331 }
332 std::shared_ptr<ParamType> buffer(new ParamType[count], std::default_delete<ParamType[]>());
333 ParamType* raw = buffer.get();
334 paramBuffers.push_back(buffer);Lines 335-344 335 return raw;
336 }
337
338 size_t CheckedAddSize(size_t left, size_t right, const char* errorMessage) {
! 339 if (left > std::numeric_limits<size_t>::max() - right) {
! 340 ThrowStdException(errorMessage);
341 }
342 return left + right;
343 }Lines 342-351 342 return left + right;
343 }
344
345 size_t CheckedMultiplySize(size_t left, size_t right, const char* errorMessage) {
! 346 if (left != 0 && right > std::numeric_limits<size_t>::max() / left) {
! 347 ThrowStdException(errorMessage);
348 }
349 return left * right;
350 }Lines 350-359 350 }
351
352 size_t CheckedFetchColumnSize(SQLULEN columnSize) {
353 const size_t result = static_cast<size_t>(columnSize);
! 354 if (static_cast<SQLULEN>(result) != columnSize) {
! 355 ThrowStdException("Column size is too large");
356 }
357 return result;
358 }Lines 359-368 359
360 SQLLEN CheckedFetchBufferLength(size_t elementCount, size_t elementSize) {
361 const size_t byteCount =
362 CheckedMultiplySize(elementCount, elementSize, "Column fetch stride is too large");
! 363 if (byteCount > static_cast<size_t>(std::numeric_limits<SQLLEN>::max())) {
! 364 ThrowStdException("Column fetch stride is too large");
365 }
366 return static_cast<SQLLEN>(byteCount);
367 }Lines 421-431 421 case SQL_C_CHAR:
422 case SQL_C_BINARY:
423 return CheckedAddSize(info.columnSize, 1, "Character parameter size is too large");
424 case SQL_C_BIT:
! 425 return sizeof(char);
! 426 case SQL_C_STINYINT:
! 427 case SQL_C_USHORT:
428 return sizeof(unsigned short);
429 case SQL_C_SBIGINT:
430 case SQL_C_SLONG:
431 case SQL_C_UBIGINT:Lines 433-456 433 return sizeof(int64_t);
434 case SQL_C_FLOAT:
435 return sizeof(float);
436 case SQL_C_TYPE_DATE:
! 437 return sizeof(SQL_DATE_STRUCT);
! 438 case SQL_C_TYPE_TIME:
439 return sizeof(SQL_TIME_STRUCT);
440 case SQL_C_TYPE_TIMESTAMP:
441 return sizeof(SQL_TIMESTAMP_STRUCT);
442 case SQL_C_SS_TIMESTAMPOFFSET:
! 443 return sizeof(DateTimeOffset);
! 444 case SQL_C_NUMERIC:
445 return sizeof(SQL_NUMERIC_STRUCT);
446 case SQL_C_GUID:
447 return sizeof(SQLGUID);
448 case SQL_C_DEFAULT:
! 449 return sizeof(char);
! 450 default:
451 ThrowStdException("Unsupported C type for parameter array allocation");
! 452 }
453 return 0;
454 }
455
456 template <typename ElementType>Lines 460-469 460 CheckedMultiplySize(rowIndex, rowStride, "Arrow source offset is too large");
461 const size_t rowCapacity = CheckedMultiplySize(
462 rowStride, sizeof(ElementType), "Arrow source capacity is too large");
463 if (offset > buffer.size() || rowStride > buffer.size() - offset ||
! 464 dataBytes > rowCapacity) {
! 465 ThrowStdException("Driver data length exceeds the allocated fetch buffer");
466 }
467 return offset;
468 }Lines 468-477 468 }
469
470 template <typename ElementType>
471 std::unique_ptr<ElementType[]> AllocateUniqueArray(size_t count, const char* errorMessage) {
! 472 if (count > std::numeric_limits<size_t>::max() / sizeof(ElementType)) {
! 473 ThrowStdException(errorMessage);
474 }
475 return std::make_unique<ElementType[]>(count);
476 }Lines 560-568 560 const char* errorMessage) {
561 ReserveNativeFetchBytes(reservedBytes, count, sizeof(ElementType));
562 return AllocateUniqueArray<ElementType>(count, errorMessage);
563 }
! 564
565 std::string DescribeChar(unsigned char ch) {
566 if (ch >= 32 && ch <= 126) {
567 return std::string("'") + static_cast<char>(ch) + "'";
568 } else {Lines 603-612 603 if (!chunk) throw py::error_already_set();
604 py::object encoded = encoder.attr("encode")(chunk, end == length);
605 char* data = nullptr;
606 Py_ssize_t size = 0;
! 607 if (PyBytes_AsStringAndSize(encoded.ptr(), &data, &size) != 0) {
! 608 throw py::error_already_set();
609 }
610 if (size > 0) {
611 SQLRETURN rc = put_data_fn(data, static_cast<SQLLEN>(size));
612 if (!SQL_SUCCEEDED(rc)) return rc;Lines 611-627 611 SQLRETURN rc = put_data_fn(data, static_cast<SQLLEN>(size));
612 if (!SQL_SUCCEEDED(rc)) return rc;
613 }
614 }
! 615 if (length == 0) {
! 616 py::object encoded = encoder.attr("encode")(py::str(), true);
! 617 char* data = nullptr;
! 618 Py_ssize_t size = 0;
! 619 if (PyBytes_AsStringAndSize(encoded.ptr(), &data, &size) != 0) {
! 620 throw py::error_already_set();
! 621 }
! 622 if (size > 0) return put_data_fn(data, static_cast<SQLLEN>(size));
! 623 return put_data_fn(nullptr, 0);
624 }
625 return SQL_SUCCESS;
626 }Lines 633-643 633
634 static SQLRETURN StreamDAEParameter(SQLHSTMT hStmt, const ParamInfo& info,
635 const std::string& charEncoding) {
636 PyObject* value = info.dataPtr.ptr();
! 637 if (!value || value == Py_None) {
! 638 py::gil_scoped_release release;
! 639 return SQLPutData_ptr(hStmt, nullptr, 0);
640 }
641
642 auto putImmutableData = [&](SQLPOINTER data, SQLLEN length) {
643 py::gil_scoped_release release;Lines 645-657 645 };
646 if (PyUnicode_Check(value)) {
647 if (info.paramCType == SQL_C_WCHAR) {
648 return stream_unicode_dae_chunks(value, "utf-16-le", putImmutableData);
! 649 }
! 650 if (info.paramCType == SQL_C_CHAR) {
! 651 return stream_unicode_dae_chunks(value, charEncoding, putImmutableData);
! 652 }
! 653 ThrowStdException("DAE only supports text C types for str values");
654 }
655 if (PyBytes_Check(value)) {
656 return stream_dae_chunks(PyBytes_AS_STRING(value),
657 static_cast<size_t>(PyBytes_GET_SIZE(value)),Lines 663-672 663 std::vector<char> chunk(std::min(static_cast<size_t>(DAE_CHUNK_SIZE), totalBytes));
664 for (size_t offset = 0; offset < totalBytes; offset += chunk.size()) {
665 const size_t currentSize = static_cast<size_t>(PyByteArray_GET_SIZE(value));
666 const size_t length = std::min(chunk.size(), totalBytes - offset);
! 667 if (currentSize < offset + length) {
! 668 ThrowStdException("bytearray changed size during DAE streaming");
669 }
670 std::copy_n(PyByteArray_AS_STRING(value) + offset, length, chunk.data());
671 SQLRETURN rc = putImmutableData(chunk.data(), static_cast<SQLLEN>(length));
672 if (!SQL_SUCCEEDED(rc)) return rc;Lines 671-680 671 SQLRETURN rc = putImmutableData(chunk.data(), static_cast<SQLLEN>(length));
672 if (!SQL_SUCCEEDED(rc)) return rc;
673 }
674 return SQL_SUCCESS;
! 675 }
! 676 ThrowStdException("DAE only supports str, bytes, or bytearray values");
677 return SQL_ERROR;
678 }
679
680 // GH-610: Resolve SQL type for a NULL parameter using per-handle cache.Lines 868-876 868 if (py::isinstance<py::str>(param)) {
869 // Encode Unicode string using the specified encoding
870 try {
871 py::object encoded =
! 872 EncodeUnicodeValue(param.ptr(), charEncoding);
873 encodedStr = encoded.cast<std::string>();
874 LOG("BindParameters: param[%d] SQL_C_CHAR - Encoded with '%s', "
875 "size=%zu bytes",
876 paramIndex, charEncoding.c_str(), encodedStr.size());Lines 1953-1961 1953 return; // Refuse to mark - let normal free() handle it
1954 }
1955 clearResult();
1956 _implicitly_freed = true;
! 1957 retainedDaeBindings.reset();
1958 }
1959
1960 /*
1961 * IMPORTANT: Never log in destructors - it causes segfaults.Lines 2577-2588 2577 SQLRETURN describeRc = PreResolveUdtTypes(hStmt, paramInfos);
2578 if (!SQL_SUCCEEDED(describeRc)) return describeRc;
2579 // GH-627: resolve unknown NULL array param SQL types before binding any param.
2580 PreResolveUnknownNullTypes(handle, hStmt, paramInfos);
! 2581 size_t reservedParameterBytes = 0;
! 2582 for (const ParamInfo& info : paramInfos) {
! 2583 ReserveNativeParameterBytes(reservedParameterBytes, paramSetSize,
! 2584 ParameterArrayElementSize(info));
2585 ReserveNativeParameterBytes(reservedParameterBytes, paramSetSize, sizeof(SQLLEN));
2586 }
2587 for (int paramIndex = 0; paramIndex < columnwise_params.size(); ++paramIndex) {
2588 const py::list& columnValues = columnwise_params[paramIndex].cast<py::list>();Lines 2651-2662 2651 elementWidth, sizeof(SQLWCHAR),
2652 "Wide-character parameter length is too large");
2653 if (bufferBytes > static_cast<size_t>(std::numeric_limits<SQLLEN>::max())) {
2654 ThrowStdException("Wide-character parameter length is too large");
! 2655 }
2656 SQLWCHAR* wcharArray = AllocateParamBufferArray<SQLWCHAR>(
! 2657 tempBuffers,
! 2658 CheckedMultiplySize(paramSetSize, elementWidth,
2659 "Wide-character parameter buffer is too large"));
2660 strLenOrIndArray = AllocateParamBufferArray<SQLLEN>(tempBuffers, paramSetSize);
2661 for (size_t i = 0; i < paramSetSize; ++i) {
2662 if (columnValues[i].is_none()) {Lines 2670-2679 2670 }
2671 if (static_cast<size_t>(
2672 PyUnicode_GET_LENGTH(columnValues[i].ptr())) >
2673 info.columnSize) {
! 2674 ThrowStdException("Input string exceeds allowed column size "
! 2675 "at parameter index " +
2676 std::to_string(paramIndex));
2677 }
2678 py::object encoded =
2679 EncodeUnicodeValue(columnValues[i].ptr(), "utf-16-le");Lines 2705-2713 2705 LOG("BindParameterArray: SQL_C_WCHAR bound - "
2706 "param_index=%d",
2707 paramIndex);
2708 dataPtr = wcharArray;
! 2709 bufferLength = static_cast<SQLLEN>(bufferBytes);
2710 break;
2711 }
2712 case SQL_C_TINYINT:
2713 case SQL_C_UTINYINT: {Lines 2776-2788 2776 case SQL_C_BINARY: {
2777 LOG("BindParameterArray: Binding SQL_C_CHAR/BINARY array - "
2778 "param_index=%d, count=%zu, column_size=%zu, encoding='%s'",
2779 paramIndex, paramSetSize, info.columnSize, charEncoding.c_str());
! 2780 const size_t elementWidth = CheckedAddSize(
! 2781 info.columnSize, 1, "Character parameter size is too large");
! 2782 if (elementWidth > static_cast<size_t>(std::numeric_limits<SQLLEN>::max())) {
! 2783 ThrowStdException("Character parameter length is too large");
! 2784 }
2785 char* charArray = AllocateParamBufferArray<char>(
2786 tempBuffers,
2787 CheckedMultiplySize(paramSetSize, elementWidth,
2788 "Character parameter buffer is too large"));Lines 2792-2800 2792 strLenOrIndArray[i] = SQL_NULL_DATA;
2793 std::memset(charArray + i * (info.columnSize + 1), 0,
2794 info.columnSize + 1);
2795 } else {
! 2796 if (info.paramCType == SQL_C_BINARY &&
2797 !py::isinstance<py::bytes>(columnValues[i]) &&
2798 !py::isinstance<py::bytearray>(columnValues[i])) {
2799 ThrowStdException(MakeParamMismatchErrorStr(info.paramCType,
2800 paramIndex));Lines 2809-2818 2809 encoded =
2810 EncodeUnicodeValue(columnValues[i].ptr(), charEncoding);
2811 if (PyBytes_AsStringAndSize(encoded.ptr(), &encodedData,
2812 &encodedSize) != 0) {
! 2813 throw py::error_already_set();
! 2814 }
2815 LOG("BindParameterArray: param[%d] row[%zu] SQL_C_CHAR - "
2816 "Encoded with '%s', "
2817 "size=%zu bytes",
2818 paramIndex, i, charEncoding.c_str(),Lines 2843-2861 2843 ThrowStdException(
2844 MakeParamMismatchErrorStr(info.paramCType, paramIndex));
2845 }
2846
! 2847 const size_t dataSize = static_cast<size_t>(encodedSize);
! 2848 if (dataSize > info.columnSize) {
2849 LOG("BindParameterArray: String/binary too "
2850 "long - param_index=%d, row=%zu, size=%zu, "
2851 "max=%zu",
! 2852 paramIndex, i, dataSize, info.columnSize);
2853 ThrowStdException("Input exceeds column size at index " +
2854 std::to_string(i));
2855 }
! 2856 std::copy_n(encodedData, dataSize, charArray + i * elementWidth);
! 2857 strLenOrIndArray[i] = static_cast<SQLLEN>(dataSize);
2858 }
2859 }
2860 LOG("BindParameterArray: SQL_C_CHAR/BINARY bound - "
2861 "param_index=%d",Lines 2860-2868 2860 LOG("BindParameterArray: SQL_C_CHAR/BINARY bound - "
2861 "param_index=%d",
2862 paramIndex);
2863 dataPtr = charArray;
! 2864 bufferLength = static_cast<SQLLEN>(elementWidth);
2865 break;
2866 }
2867 case SQL_C_BIT: {
2868 LOG("BindParameterArray: Binding SQL_C_BIT array - "Lines 3279-3295 3279 class DaeRowBindingGuard {
3280 public:
3281 DaeRowBindingGuard(const SqlHandlePtr& handle, const std::vector<ParamInfo>& infos)
3282 : bindings(std::make_shared<DaeRowBindings>()), handle(handle) {
! 3283 if (handle->retainedDaeBindings) {
! 3284 const SQLRETURN rc = SQLFreeStmt_ptr(handle->get(), SQL_RESET_PARAMS);
! 3285 CheckFetchError(handle, rc);
3286 handle->retainedDaeBindings.reset();
3287 }
3288 bindings->infos = infos;
! 3289 handle->retainedDaeBindings = bindings;
! 3290 }
! 3291
3292 ~DaeRowBindingGuard() noexcept {
3293 if (!finished) {
3294 SQLRETURN cancelRc;
3295 SQLRETURN resetRc;Lines 3293-3304 3293 if (!finished) {
3294 SQLRETURN cancelRc;
3295 SQLRETURN resetRc;
3296 {
! 3297 py::gil_scoped_release release;
! 3298 cancelRc = SQLCancel_ptr(handle->get());
! 3299 resetRc = SQLFreeStmt_ptr(handle->get(), SQL_RESET_PARAMS);
! 3300 }
3301 if (!SQL_SUCCEEDED(cancelRc)) {
3302 std::fputs("mssql-python: failed to cancel abandoned DAE execution\n", stderr);
3303 }
3304 if (SQL_SUCCEEDED(resetRc)) {Lines 3310-3320 3310 }
3311 // Retained storage may be released by GIL-less connection cleanup.
3312 for (auto& info : bindings->infos) {
3313 info.dataPtr = py::object();
! 3314 }
! 3315 }
! 3316
3317 void finish() {
3318 const SQLRETURN rc = SQLFreeStmt_ptr(handle->get(), SQL_RESET_PARAMS);
3319 CheckFetchError(handle, rc);
3320 finished = true;Lines 3426-3435 3426 DaeRowBindingGuard rowBindings(statementHandle, paramInfos);
3427 auto& rowParamInfos = rowBindings.bindings->infos;
3428 for (size_t paramIndex = 0; paramIndex < rowParamInfos.size(); ++paramIndex) {
3429 if (rowParams[paramIndex].is_none()) {
! 3430 rowParamInfos[paramIndex].paramCType = SQL_C_DEFAULT;
! 3431 rowParamInfos[paramIndex].isDAE = false;
3432 rowParamInfos[paramIndex].dataPtr = py::none();
3433 }
3434 }
3435 rc = BindParameters(*statementHandle, hStmt, rowParams, rowParamInfos,Lines 3464-3474 3464 dae_chunk_count, rc);
3465 CheckFetchError(statementHandle, rc);
3466 return rc;
3467 }
! 3468 if (rc != SQL_NEED_DATA) break;
! 3469
! 3470 auto matchedInfo = std::find_if(
3471 rowParamInfos.begin(), rowParamInfos.end(),
3472 [token](const ParamInfo& info) { return token == &info; });
3473 if (matchedInfo == rowParamInfos.end()) {
3474 ThrowStdException(Lines 3645-3657 3645
3646 std::shared_ptr<PendingArrowRows> PendingRowsForNextFetch(SqlHandlePtr handle) {
3647 auto pending = handle->pendingArrowRows();
3648 if (pending) {
! 3649 pending->hasSelectedRow = false;
3650 if (pending->next == pending->count) {
3651 handle->setPendingArrowRows(nullptr);
3652 return nullptr;
! 3653 }
3654 }
3655 return pending;
3656 }Lines 3655-3663 3655 return pending;
3656 }
3657
3658 SQLRETURN ReadSelectedArrowRow(SqlHandlePtr handle, PendingArrowRows& pending,
! 3659 SQLUSMALLINT columnCount, py::list& row,
3660 const std::string& charEncoding, py::handle messages);
3661
3662 // Wrap SQLFetch to retrieve rows
3663 SQLRETURN SQLFetch_wrap(SqlHandlePtr StatementHandle) {Lines 3688-3698 3688 bool isWideChar, bool isBinary, const std::string& charEncoding,
3689 py::handle messages) {
3690 PERF_TIMER("FetchLobColumnData");
3691 std::vector<char> buffer;
! 3692 size_t dataSize = 0;
! 3693 size_t reservedBytes = 0;
! 3694 ReserveNativeFetchBytes(reservedBytes, DAE_CHUNK_SIZE, sizeof(char));
3695 SQLRETURN ret = SQL_SUCCESS_WITH_INFO;
3696 int loopCount = 0;
3697
3698 while (true) {Lines 3779-3791 3779 }
3780 return py::str("");
3781 }
3782 if (isWideChar) {
! 3783 if (dataSize % sizeof(SQLWCHAR) != 0) {
! 3784 ThrowStdException("Wide-character LOB data has an invalid byte length");
! 3785 }
! 3786 int byteOrder = -1;
! 3787 PyObject* decoded =
3788 PyUnicode_DecodeUTF16(buffer.data(), static_cast<Py_ssize_t>(dataSize),
3789 "strict", &byteOrder);
3790 if (decoded == nullptr) {
3791 throw py::error_already_set();Lines 3885-3893 3885 PERF_TIMER("SQLGetData_wrap");
3886 // Note: wcharEncoding parameter is reserved for future use
3887 // Currently WCHAR data always uses UTF-16LE for Windows compatibility
3888 (void)wcharEncoding; // Suppress unused parameter warning
! 3889 if (auto pending = StatementHandle->pendingArrowRows()) {
3890 if (!pending->hasSelectedRow) {
3891 ThrowStdException("Pending Arrow data requires a successful DDBCSQLFetch or "
3892 "DDBCSQLFetchScroll before DDBCSQLGetData");
3893 }Lines 4851-4859 4851 ResizeNativeFetchBuffer(
4852 buffers.charBuffers[col - 1],
4853 CheckedMultiplySize(fetchSize, MAX_DIGITS_IN_NUMERIC,
4854 "Native fetch buffer is too large"),
! 4855 reservedBytes);
4856 ret = SQLBindCol_ptr(hStmt, col, SQL_C_CHAR, buffers.charBuffers[col - 1].data(),
4857 MAX_DIGITS_IN_NUMERIC * sizeof(SQLCHAR),
4858 buffers.indicators[col - 1].data());
4859 break;Lines 4908-4921 4908 CheckedMultiplySize(fetchSize, fetchBufferSize,
4909 "Native fetch buffer is too large"),
4910 reservedBytes);
4911 ret = SQLBindCol_ptr(hStmt, col, SQL_C_BINARY, buffers.charBuffers[col - 1].data(),
! 4912 CheckedFetchBufferLength(fetchBufferSize, 1),
! 4913 buffers.indicators[col - 1].data());
4914 break;
4915 }
4916 case SQL_SS_TIMESTAMPOFFSET:
! 4917 ResizeNativeFetchBuffer(buffers.datetimeoffsetBuffers[col - 1], fetchSize,
4918 reservedBytes);
4919 ret = SQLBindCol_ptr(hStmt, col, SQL_C_SS_TIMESTAMPOFFSET,
4920 buffers.datetimeoffsetBuffers[col - 1].data(),
4921 sizeof(DateTimeOffset),Lines 4952-4960 4952 py::list& rows, SQLUSMALLINT numCols, SQLULEN& numRowsFetched,
4953 const std::vector<SQLUSMALLINT>& lobColumns,
4954 const std::string& charEncoding = "utf-16le", int charCtype = SQL_C_WCHAR,
4955 py::handle messages = {}, SQLULEN firstRow = 0,
! 4956 int bufferRows = 0) {
4957 // Pre-cache column metadata to avoid repeated dictionary lookups.
4958 // The vectors below are consumed later by construct_rows, so they are
4959 // declared at function scope; only the population work is wrapped in the
4960 // cache_column_metadata timer's block (an earlier version put the timer atLines 5299-5307 5299 // Mark guard as released so destructor doesn't double-free
5300 guard.release();
5301 Py_DECREF(row);
5302 }
! 5303 return SQL_SUCCESS;
5304 }
5305
5306 template <typename Metadata>
5307 SQLRETURN FetchBatchData(SQLHSTMT hStmt, ColumnBuffers& buffers, const Metadata& columnNames,Lines 5337-5345 5337 for (SQLUSMALLINT col = 1; col <= numCols; col++) {
5338 auto columnMeta = columnNames[col - 1].cast<py::dict>();
5339 SQLSMALLINT dataType = columnMeta["DataType"].cast<SQLSMALLINT>();
5340 SQLULEN columnSize = columnMeta["ColumnSize"].cast<SQLULEN>();
! 5341 size_t columnBytes = 0;
5342
5343 switch (dataType) {
5344 case SQL_CHAR:
5345 case SQL_VARCHAR:Lines 5374-5382 5374 case SQL_INTEGER:
5375 columnBytes = sizeof(SQLINTEGER);
5376 break;
5377 case SQL_SMALLINT:
! 5378 columnBytes = sizeof(SQLSMALLINT);
5379 break;
5380 case SQL_REAL:
5381 columnBytes = sizeof(SQLREAL);
5382 break;Lines 5414-5422 5414 case SQL_SS_UDT:
5415 case SQL_BINARY:
5416 case SQL_VARBINARY:
5417 case SQL_LONGVARBINARY:
! 5418 HandleZeroColumnSizeAtFetch(columnSize);
5419 columnBytes = columnSize;
5420 break;
5421 case SQL_SS_TIMESTAMPOFFSET:
5422 columnBytes = sizeof(DateTimeOffset);Lines 5803-5811 5803 return ReadArrowScalar(stmt, ordinal, SQL_C_CHAR, buffers.charBuffers[column],
5804 indicator, reservedBytes, messages, MAX_DIGITS_IN_NUMERIC);
5805 case SQL_DOUBLE:
5806 case SQL_FLOAT:
! 5807 return ReadArrowScalar(stmt, ordinal, SQL_C_DOUBLE, buffers.doubleBuffers[column],
5808 indicator, reservedBytes, messages);
5809 case SQL_TIMESTAMP:
5810 case SQL_TYPE_TIMESTAMP:
5811 case SQL_DATETIME:Lines 5841-5856 5841 size_t reservedBytes = pending.reservedBytes;
5842 for (; pending.lobColumn < columnCount; ++pending.lobColumn) {
5843 auto column = columns[pending.lobColumn].cast<py::dict>();
5844 const SQLRETURN ret = ReadArrowLobColumn(handle->get(), pending,
! 5845 column["DataType"].cast<SQLSMALLINT>(),
! 5846 pending.lobColumn, reservedBytes, messages);
! 5847 CheckFetchError(handle, ret);
! 5848 if (!SQL_SUCCEEDED(ret)) {
! 5849 return ret;
! 5850 }
! 5851 }
! 5852 }
5853 return SQL_SUCCESS;
5854 }
5855
5856 SQLRETURN ReadSelectedArrowRow(SqlHandlePtr handle, PendingArrowRows& pending,Lines 5870-5879 5870 }
5871 py::list rows;
5872 SQLULEN end = pending.selectedRow + 1;
5873 ret = ConvertBufferedRows(handle->get(), pending.buffers, columns, rows, columnCount, end,
! 5874 {}, charEncoding, pending.charCtype, messages,
! 5875 pending.selectedRow, pending.fetchSize);
5876 if (SQL_SUCCEEDED(ret)) {
5877 for (auto value : rows[0].cast<py::list>()) {
5878 row.append(value);
5879 }Lines 5900-5916 5900 SQLULEN end = pending->next + std::min(limit, pending->count - pending->next);
5901 ret = ConvertBufferedRows(handle->get(), pending->buffers, columns, rows, numCols, end,
5902 {}, charEncoding, pending->charCtype, messages,
5903 pending->next, pending->fetchSize);
! 5904 if (SQL_SUCCEEDED(ret)) {
! 5905 pending->next = end;
! 5906 if (pending->next == pending->count) {
5907 handle->setPendingArrowRows(nullptr);
5908 }
5909 }
5910 return ret;
! 5911 }
! 5912
5913 int32_t days_from_civil(int y, int m, int d) {
5914 // Implements the "days_from_civil" algorithm by Howard Hinnant
5915 // Returns number of days since Unix epoch (1970-01-01)
5916 y -= m <= 2;Lines 5922-5930 5922 }
5923
5924 SQLRETURN FetchArrowBatch_wrap(SqlHandlePtr StatementHandle, py::list& capsules,
5925 py::handle arrowBatchSizeArg, int charCtype,
! 5926 py::handle messages = {}) {
5927 PERF_TIMER("FetchArrowBatch_wrap");
5928 const int arrowBatchSize =
5929 ValidateNativeRowCountArgument(arrowBatchSizeArg, "Arrow batch size", true);
5930 const size_t batchSize = static_cast<size_t>(arrowBatchSize);Lines 5931-5939 5931 const size_t offsetCount = CheckedAddSize(batchSize, 1, "Arrow batch size is too large");
5932 const size_t initialVarDataSize =
5933 CheckedMultiplySize(batchSize, 42, "Arrow batch size is too large");
5934 const size_t bitmapSize =
! 5935 CheckedAddSize(batchSize, 7, "Arrow batch size is too large") / 8;
5936 // Fetch narrow char data as SQL_C_CHAR if on Linux/macOS and configured by the user
5937 charCtype = EffectiveCharCtypeForFetch(charCtype, "utf-8");
5938 if (arrowBatchSize > 0) {
5939 PendingRowsForNextFetch(StatementHandle);Lines 5969-5977 5969 std::vector<SQLSMALLINT> dataTypes(numCols);
5970 std::vector<bool> columnNullable(numCols);
5971 std::vector<bool> columnVarLen(numCols, false);
5972 std::vector<int64_t> nullCounts(numCols, 0);
! 5973 size_t reservedBytes = pending ? pending->reservedBytes : 0;
5974
5975 std::vector<std::unique_ptr<ArrowArrayPrivateData>> arrowArrayPrivateData(numCols);
5976 std::vector<std::unique_ptr<ArrowSchemaPrivateData>> arrowSchemaPrivateData(numCols);
5977 for (SQLSMALLINT i = 0; i < numCols; i++) {Lines 6031-6039 6031 arrowColumnProducer->varVal =
6032 AllocateArrowArray<uint64_t>(offsetCount, reservedBytes,
6033 "Arrow offset buffer is too large");
6034 EnsureNativeFetchBufferSize(arrowColumnProducer->varData, initialVarDataSize,
! 6035 reservedBytes);
6036 columnVarLen[i] = true;
6037 // start at offset 0
6038 arrowColumnProducer->varVal[0] = 0;
6039 arrowColumnProducer->ptrValueBuffer = arrowColumnProducer->varVal.get();Lines 6190-6198 6190
6191 if (!hasLobColumns && bufferFetchSize > 0) {
6192 const size_t before = reservedBytes;
6193 ret = SQLBindColums(hStmt, buffers, columnNames, numCols, fetchSize, reservedBytes,
! 6194 charCtype, messages);
6195 pending->reservedBytes += reservedBytes - before;
6196 if (!SQL_SUCCEEDED(ret)) {
6197 LOG("Error when binding columns");
6198 return ret;Lines 6255-6269 6255 ThrowStdException("Unexpected negative data length.");
6256 }
6257 size_t bytes = static_cast<size_t>(indicator);
6258 const auto type = dataTypes[col];
! 6259 if (type == SQL_GUID) {
! 6260 bytes = 37;
! 6261 } else if (type == SQL_WCHAR || type == SQL_WVARCHAR ||
! 6262 type == SQL_WLONGVARCHAR || type == SQL_SS_XML ||
! 6263 ((type == SQL_CHAR || type == SQL_VARCHAR ||
! 6264 type == SQL_LONGVARCHAR) && charCtype == SQL_C_WCHAR)) {
! 6265 if (bytes % sizeof(SQLWCHAR) != 0) {
6266 ThrowStdException("Wide-character data has an invalid byte length");
6267 }
6268 const auto& source = buffers.wcharBuffers[col];
6269 const size_t stride = source.size() / pending->fetchSize;Lines 6350-6359 6350 static_cast<size_t>(fetchSize);
6351 const size_t sourceOffset = CheckedArrowSourceOffset(
6352 buffers.charBuffers[idxCol], idxRowSql, sourceStride, dataLen);
6353
! 6354 std::memcpy(&(*target_vec)[start],
! 6355 &buffers.charBuffers[idxCol][sourceOffset], dataLen);
6356 }
6357 arrowColumnProducer->varVal[idxRowArrow + 1] = start + dataLen;
6358 break;
6359 }mssql_python/pybind/ddbc_bindings.hmssql_python/pybind/param_detect.hppLines 141-158 141 inline constexpr int MAX_INLINE_BINARY = 8000;
142
143 inline SQLULEN DAEColumnSize(SQLSMALLINT sqlType, SQLULEN actualSize) {
144 switch (sqlType) {
! 145 case SQL_CHAR:
146 case SQL_VARCHAR:
147 return actualSize > MAX_INLINE_CHAR ? 0 : actualSize;
! 148 case SQL_WCHAR:
149 case SQL_WVARCHAR:
150 return actualSize > MAX_INLINE_CHAR ? 0 : actualSize;
! 151 case SQL_BINARY:
152 case SQL_VARBINARY:
153 return actualSize > MAX_INLINE_BINARY ? 0 : actualSize;
! 154 case SQL_LONGVARCHAR:
155 case SQL_WLONGVARCHAR:
156 case SQL_LONGVARBINARY:
157 return actualSize;
158 default:Lines 237-252 237 if (!chunk) throw py::error_already_set();
238 py::object encoded = encoder.attr("encode")(chunk, end == length);
239 const Py_ssize_t encodedSize = PyBytes_GET_SIZE(encoded.ptr());
240 if (encodedSize > MAX_INLINE_BINARY - total) {
! 241 return MAX_INLINE_BINARY + 1;
! 242 }
243 total += encodedSize;
244 }
245 if (length == 0) {
! 246 py::object encoded = encoder.attr("encode")(py::str(), true);
! 247 total = PyBytes_GET_SIZE(encoded.ptr());
! 248 }
249 return total;
250 }
251
252 inline PyObject* FormatDecimalParam(PyObject* params, Py_ssize_t index, PyObject* value) {Lines 275-290 275 }
276
277 inline long long ValidatedInputSizeInteger(PyObject* value, const char* fieldName) {
278 if (!PyLong_Check(value) || PyBool_Check(value)) {
! 279 throw py::type_error(std::string(fieldName) + " must be an integer");
! 280 }
281 int overflow = 0;
282 const long long result = PyLong_AsLongLongAndOverflow(value, &overflow);
283 if ((result == -1 && PyErr_Occurred()) || overflow != 0) {
! 284 PyErr_Clear();
! 285 throw py::value_error(std::string(fieldName) + " is out of range");
! 286 }
287 return result;
288 }
289
290 inline void ValidateInputSizes(PyObject* inputSizes) {Lines 316-325 316 if (sqlType < std::numeric_limits<SQLSMALLINT>::min() ||
317 sqlType > std::numeric_limits<SQLSMALLINT>::max() ||
318 cType < std::numeric_limits<SQLSMALLINT>::min() ||
319 cType > std::numeric_limits<SQLSMALLINT>::max()) {
! 320 throw py::value_error("SQL and C types must fit in SQLSMALLINT");
! 321 }
322 const bool isNumeric = sqlType == SQL_DECIMAL || sqlType == SQL_NUMERIC;
323 py::int_ zero(0);
324 const int negativeColumnSize =
325 PyObject_RichCompareBool(columnSize, zero.ptr(), Py_LT);Lines 325-340 325 PyObject_RichCompareBool(columnSize, zero.ptr(), Py_LT);
326 const int negativeDecimalDigits =
327 PyObject_RichCompareBool(decimalDigits, zero.ptr(), Py_LT);
328 if (negativeColumnSize == -1 || negativeDecimalDigits == -1) {
! 329 throw py::error_already_set();
! 330 }
331 if (negativeColumnSize == 1) {
332 throw py::value_error("column size must be non-negative");
333 }
334 if (negativeDecimalDigits == 1) {
! 335 throw py::value_error("decimal digits must be non-negative");
! 336 }
337 if (!isNumeric) {
338 const unsigned long long requestedSize = PyLong_AsUnsignedLongLong(columnSize);
339 if (requestedSize == static_cast<unsigned long long>(-1) && PyErr_Occurred()) {
340 PyErr_Clear();Lines 340-349 340 PyErr_Clear();
341 throw py::value_error("column size is out of range");
342 }
343 if (requestedSize > std::numeric_limits<SQLULEN>::max()) {
! 344 throw py::value_error("column size is out of range");
! 345 }
346 const unsigned long long requestedDigits =
347 PyLong_AsUnsignedLongLong(decimalDigits);
348 if (requestedDigits == static_cast<unsigned long long>(-1) && PyErr_Occurred()) {
349 PyErr_Clear();Lines 350-359 350 throw py::value_error("decimal digits are out of range");
351 }
352 if (requestedDigits >
353 static_cast<unsigned long long>(std::numeric_limits<SQLSMALLINT>::max())) {
! 354 throw py::value_error("decimal digits are out of range");
! 355 }
356 }
357 }
358 }Lines 477-486 477 const std::string& charEncoding = "utf-8") {
478 PyTypeCache::initialize();
479
480 if (!PyList_Check(params)) {
! 481 throw py::type_error("params must be a list");
! 482 }
483 ValidateInputSizes(inputSizes);
484
485 const Py_ssize_t n = PyList_GET_SIZE(params);
486 const Py_ssize_t inputSizeCount = inputSizes == Py_None ? 0 : PyList_GET_SIZE(inputSizes);📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.performance_counter.hpp: 0.7%
mssql_python.pybind.logger_bridge.cpp: 57.9%
mssql_python.pybind.ddbc_bindings.h: 65.6%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.connection.connection.cpp: 83.2%
mssql_python.pybind.ddbc_bindings.cpp: 83.3%
mssql_python.pybind.connection.connection_pool.cpp: 83.5%
mssql_python.logging.py: 86.2%
mssql_python.pooling.py: 90.1%
mssql_python.pybind.param_detect.hpp: 90.2%🔗 Quick Links
|
…nto saumya/native-size-validation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Critical native buffer-copy and allocation risks, plus incomplete public batch-size validation, remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 2
github-actions
Bot
added
pr-size: large
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The native binding and fetch changes span several high-risk paths, and the remaining issues need resolution and cross-platform runtime validation.
1 open finding
2 resolved since last review
Previously missed (2)
In code that hasn't changed since last review
mssql_python/cursor.py:2652
The executemany size scan only includes existing str values, but text-bound datetime.time values are converted with isoformat() later, in the processed_parameters loop (cursor.py:2844-2847). With setinputsizes([(SQL_WVARCHAR, 1, 0)]) and a time subclass whose isoformat() returns more than 4,000 UTF-16 units, isDAE stays false; the array binder rejects the normalized value against the declared size instead of streaming it. Measure the normalized text before deciding on array binding, as scalar execute already does, and add a batch regression test.
mssql_python/cursor.py:3040
Validating size here makes synchronous fetchmany(-1) raise ValueError, where it previously returned [] before reaching native code. AsyncCursor.fetchmany(-1) still returns [] (async_query/async_fetch.py:91-95), and its existing test asserts this (tests/AsyncTest/test_007_async_fetch.py:654-655). Preserve the synchronous non-positive short-circuit if that is the intended API contract, or update both cursor APIs and their tests together; rejecting a value that never crosses the native boundary need not create this inconsistency.
🧠 Review effort: Lite
Encode string subclasses from their immutable Unicode contents so overridden encode methods cannot bypass native sizing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Broad native binding and streaming changes need cross-platform runtime review, and automatic time normalization still has an unresolved size gap.
0 open findings
1 resolved since last review
Previously missed (1)
In code that hasn't changed since last review
mssql_python/pybind/param_detect.hpp:620
The new size-based streaming decision only runs for values that arrive as str. Without setinputsizes, DetectParamTypes handles a datetime.time separately: it calls isoformat() and returns without measuring the resulting text. A time subclass whose isoformat() returns a very large string therefore leaves isDAE false, and BindParameters copies that string into an unbounded native UTF-16 buffer. Check the normalized value in the automatic time path too, rejecting oversized text if SQL_TYPE_TIME cannot be streamed safely. The new large-time test covers only an explicit text override.
🧠 Review effort: Lite
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Two executemany sizing issues remain, and the broad native streaming changes need cross-platform runtime review.
1 open finding
Previously missed (1)
In code that hasn't changed since last review
mssql_python/cursor.py:2670
With setinputsizes([(SQL_VARCHAR, 1, 0)]), an executemany row containing a datetime.time subclass whose isoformat() returns 4,001 characters is not included in this str-only scan. The later conversion turns it into text, but array binding has already been selected and rejects its length against the declared size; the same value works through scalar execute, which normalizes before choosing streaming. Normalize and size time values before selecting array binding, or route such rows through scalar execution.
🧠 Review effort: Lite
Bypass string subclass protocol overrides during bounded narrow and UTF-16 executemany sizing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The broad native binding and fetch changes need cross-platform human validation, and a binary-subclass sizing issue remains unresolved.
0 open findings
1 resolved since last review
Previously missed (1)
In code that hasn't changed since last review
mssql_python/cursor.py:2679
Both the new explicit-size scan here and the automatic binary scan at cursor.py:2808-2815 use overridable len(value). A bytes or bytearray subclass can report length 1 while holding 8,001 bytes. executemany() then leaves DAE disabled; the native array binder reads the real buffer size and rejects the row as exceeding its allocated column size, although the scalar path can stream it. Measure the underlying buffer length in both scans so the actual payload selects streaming.
🧠 Review effort: Lite
Sumit Sarabhai (sumitmsft)
left a comment
Sumit Sarabhai (sumitmsft)
left a comment
Contributor
There was a problem hiding this comment.
Revalidated the four previously drafted comments against bf5e738. The row/tuple comparison assertion is fixed, so I have omitted that comment. This is a targeted revalidation of those comments, not a new full review.
Document the compatibility changes
Please document the new 1,000,000-row caps (including bulk-copy rows per commit), the native allocation budgets, and changed negative/boolean-size behavior in API documentation and the changelog. Explain how parameter-array limits depend on batch size and column widths, with guidance for reducing batch sizes. Distinguish native buffer limits from total process or result memory.
Reject oversized formatted numeric batch parameters before native binding. Cover the rejection boundary and the existing wide automatic text binding, and document row caps, allocation budgets, and compatibility changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
gargsaumya
commented
Oct 19, 2026
gargsaumya
commented
Oct 19, 2026
Use dedicated context-managed cursors so expected binding exceptions cannot leave input-size hints on the shared module cursor. Verified the reported binary-rejection then numeric-test ordering: eight cases pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use budget-aware amortized growth and lossless partial Arrow batches with statement-owned pending rows and resumable LOB reads. Preserve pending state across mixed fetch, raw fetch/GetData, scroll/skip, and metadata-only attribute changes; clear it on real result invalidation. Document short-batch semantics and add isolated ordinary/LOB and lifecycle regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
AB#47843 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
AB#47843 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Restore 8192 rows in the isolated pending-row regressions: 4096 rows fit within the native budget and cannot exercise pending-state recovery. Reproduced the failing assertion against local SQL Server, then verified all 20 affected cases and the Arrow, execution parity, and pooling suites (303 passed, 1 skipped, 1 deselected). AB#47843 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Unbind native executemany DAE rows on every exit, capture original ODBC diagnostics before cleanup, and retain native storage when reset fails. Validate tokens by exact element identity. Route automatically detected mixed text/bytes columns through scalar binding. Add isolated native failure/reuse/refcount and mixed-value regression tests. AB#47843 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
|
||
| SQLRETURN Connection::setAttribute(SQLINTEGER attribute, py::object value) { | ||
| clearResultMetadata(); | ||
| clearResultMetadata(attribute == SQL_ATTR_RESET_CONNECTION); |
Comment on lines
+500
to
+503
| { | ||
| std::vector<ElementType> resizedBuffer(count); | ||
| std::copy(buffer.begin(), buffer.end(), resizedBuffer.begin()); | ||
| buffer.swap(resizedBuffer); |
Sumit Sarabhai (sumitmsft)
approved these changes
Oct 22, 2026
Sumit Sarabhai (sumitmsft)
left a comment
Sumit Sarabhai (sumitmsft)
left a comment
Contributor
There was a problem hiding this comment.
Re-reviewed the changes through 8fa595e, including the latest DAE token-identity validation, binding ownership/error cleanup, and mixed text/binary parameter fallback. The issues raised in my earlier review are addressed, and I found no additional actionable blocker in the reviewed changes.
The Windows x64 native build and import succeeded. The focused binding, DAE failure/reuse, handle-lifetime, Arrow, and fetch regression run completed with 557 passed and 11 skipped (the expensive Arrow budget matrix was excluded from this delta run; representative budget cases were exercised during the preceding review).
Approval does not waive CI requirements. ADO build 184988 still needs a successful rerun: the inspected failed jobs stopped on GitHub checkout authentication or dependency-feed DNS resolution, rather than a product-test failure.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Work Item / Issue Reference
Summary
No tracking issue exists for this security hardening work.